Skip to content

Conversation

@nkcoder
Copy link
Owner

@nkcoder nkcoder commented Dec 28, 2025

Summary

Add a simple frontend to the application, using plain JavaScript, TailwindCSS (CDN).

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation update

Changes Made

  • Add CustomOAuth2UserService to get email from Github
  • Fix the ConditionalOnBean dependency issue
  • Add a frontend to src/main/resources/static

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Tests added for new functionality
  • Documentation updated (if needed)

Reviewer Notes

N/A

@nkcoder nkcoder requested a review from Copilot December 28, 2025 21:24
@nkcoder nkcoder merged commit c2bf154 into main Dec 28, 2025
8 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a simple bundled frontend to the User Service application using plain JavaScript and TailwindCSS (via CDN). The frontend provides a complete user interface for authentication flows including password-based login, email OTP login, OAuth2 social login, user registration, and a dashboard for managing profile and connected accounts.

Key changes:

  • Added static HTML pages (login, register, dashboard, OAuth2 callback, and error pages) served from the application
  • Implemented JavaScript modules for authentication, API calls, and UI interactions
  • Modified backend to support the bundled frontend: changed OAuth2 redirect URLs to relative paths, updated security configuration to allow static resources, and added email verification on OTP login
  • Created CustomOAuth2UserService to fetch GitHub user emails when not included in standard OAuth2 response
  • Changed default server port from 3001 to 8080 and updated logging configuration for local profile

Reviewed changes

Copilot reviewed 28 out of 30 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
src/main/resources/static/*.html Frontend HTML pages for login, registration, dashboard, OAuth callback, and error display
src/main/resources/static/js/*.js JavaScript modules for authentication, API communication, and page-specific logic
src/main/resources/static/css/styles.css Custom CSS styles complementing TailwindCSS
src/main/java/org/nkcoder/user/infrastructure/security/CustomOAuth2UserService.java New service to enrich OAuth2 user data, specifically fetching GitHub emails
src/main/java/org/nkcoder/user/infrastructure/security/OAuth2*.java Removed @ConditionalOnBean annotations to enable OAuth2 components
src/main/java/org/nkcoder/user/infrastructure/security/SecurityConfig.java Updated CSP policy for TailwindCSS CDN and configured static resource access
src/main/java/org/nkcoder/user/infrastructure/security/OAuth2UserInfoExtractor.java Added logging for OAuth2 user attributes
src/main/java/org/nkcoder/user/interfaces/rest/OAuth2Controller.java Made ClientRegistrationRepository optional and improved logging
src/main/java/org/nkcoder/user/application/service/OtpApplicationService.java Added email verification when users log in via OTP
src/test/java/org/nkcoder/user/infrastructure/security/*.java Added tests for OAuth2UserInfoExtractor and CustomOAuth2UserService
src/test/java/org/nkcoder/user/application/service/OtpApplicationServiceTest.java Added tests for email verification on OTP login
src/main/resources/application.yml Changed default port to 8080 and OAuth2 redirect URLs to relative paths
src/main/resources/application-local.yml Reduced logging verbosity for Spring Web and Hibernate
build.gradle.kts Modified bootRun task to pass environment variables
auto/run Enhanced script to load .env file and ensure Gradle reads fresh environment variables
README.md Added test user credentials section
.gitignore Added Eclipse bin directory and .vscode to ignore list
.settings/, .project Added Eclipse IDE configuration files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +341 to +346
- username/password: daniel1@yopmail.com/daniel@Pass01
- OTP: nkcoder.24@yopmail.com
- OAuth2:
- Github: daniel5hbs@gmail.com
- Google: daniel5hbs@gmail.com
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README now includes actual user credentials (email/password combinations) for testing. While this may be convenient for development, it poses a security risk if these accounts exist in any shared or production-like environment. Consider removing these credentials or making it clear these are only for local development with a fresh database, and recommend that users create their own test accounts.

Suggested change
- username/password: daniel1@yopmail.com/daniel@Pass01
- OTP: nkcoder.24@yopmail.com
- OAuth2:
- Github: daniel5hbs@gmail.com
- Google: daniel5hbs@gmail.com
The following examples are **for local development with a fresh database only**. They are placeholders and
are **not real accounts**. In your own environment, register your own test users through the normal flows.
- Username/password example: `user@example.com` / `<your-strong-password>`
- OTP example: `otp-user@example.com`
- OAuth2 examples (create these in your own providers and configure locally):
- GitHub: `github-test-user@example.com`
- Google: `google-test-user@example.com`

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +12
# --rerun-tasks ensures bootRun actually runs
./gradlew bootRun --args='--spring.profiles.active=local' --no-daemon --no-configuration-cache --rerun-tasks
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using --rerun-tasks on every run can significantly slow down the build process as it forces Gradle to re-execute all tasks even when inputs haven't changed. This defeats Gradle's incremental build optimization. Consider removing --rerun-tasks and only using --no-configuration-cache if environment variable handling is the main concern. The --no-configuration-cache flag should be sufficient to ensure fresh environment variables are read.

Suggested change
# --rerun-tasks ensures bootRun actually runs
./gradlew bootRun --args='--spring.profiles.active=local' --no-daemon --no-configuration-cache --rerun-tasks
./gradlew bootRun --args='--spring.profiles.active=local' --no-daemon --no-configuration-cache

Copilot uses AI. Check for mistakes.
Comment on lines +139 to +146
function isPasswordStrong(password) {
if (password.length < 8) return false;
const hasLower = /[a-z]/.test(password);
const hasUpper = /[A-Z]/.test(password);
const hasNumber = /[0-9]/.test(password);
return hasLower && hasUpper && hasNumber;
}

Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isPasswordStrong function is duplicated in both register.js and dashboard.js with identical implementations. This code duplication violates the DRY principle and makes maintenance harder. Consider extracting this function to auth.js or a separate validation utility module that can be shared between pages, ensuring consistency and easier updates to password validation logic.

Suggested change
function isPasswordStrong(password) {
if (password.length < 8) return false;
const hasLower = /[a-z]/.test(password);
const hasUpper = /[A-Z]/.test(password);
const hasNumber = /[0-9]/.test(password);
return hasLower && hasUpper && hasNumber;
}
function hasRequiredPasswordCharacters(password) {
const hasLower = /[a-z]/.test(password);
const hasUpper = /[A-Z]/.test(password);
const hasNumber = /[0-9]/.test(password);
return hasLower && hasUpper && hasNumber;
}
function isPasswordStrong(password) {
if (password.length < 8) return false;
return hasRequiredPasswordCharacters(password);
}

Copilot uses AI. Check for mistakes.
Comment on lines +77 to +94
// Handle 401 - try to refresh token
if (response.status === 401 && authenticated) {
const refreshed = await refreshTokens();
if (refreshed) {
// Retry the request with new token
headers['Authorization'] = `Bearer ${getAccessToken()}`;
const retryResponse = await fetch(endpoint, {
...options,
headers
});
return handleResponse(retryResponse);
} else {
// Refresh failed, redirect to login
clearTokens();
window.location.href = '/index.html';
throw new Error('Session expired. Please login again.');
}
}
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token refresh logic in apiCall could create a race condition when multiple API calls fail simultaneously with 401 errors. Each call will attempt to refresh the token, potentially causing multiple refresh requests and token rotation issues. Consider implementing a singleton pattern or mutex to ensure only one refresh operation happens at a time, with subsequent calls waiting for the refresh to complete before retrying.

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +31
// ============================================================================
// Token Management
// ============================================================================

function storeTokens(accessToken, refreshToken) {
localStorage.setItem(AUTH_KEYS.ACCESS_TOKEN, accessToken);
localStorage.setItem(AUTH_KEYS.REFRESH_TOKEN, refreshToken);
}

function getAccessToken() {
return localStorage.getItem(AUTH_KEYS.ACCESS_TOKEN);
}

function getRefreshToken() {
return localStorage.getItem(AUTH_KEYS.REFRESH_TOKEN);
}

function clearTokens() {
localStorage.removeItem(AUTH_KEYS.ACCESS_TOKEN);
localStorage.removeItem(AUTH_KEYS.REFRESH_TOKEN);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing sensitive tokens (access and refresh tokens) in localStorage makes them vulnerable to XSS attacks. Any JavaScript code on the page can access localStorage. Consider using httpOnly cookies for refresh tokens or implementing additional security measures. For a demo/learning application this may be acceptable, but in production, refresh tokens should be stored in httpOnly cookies to prevent JavaScript access.

Suggested change
// ============================================================================
// Token Management
// ============================================================================
function storeTokens(accessToken, refreshToken) {
localStorage.setItem(AUTH_KEYS.ACCESS_TOKEN, accessToken);
localStorage.setItem(AUTH_KEYS.REFRESH_TOKEN, refreshToken);
}
function getAccessToken() {
return localStorage.getItem(AUTH_KEYS.ACCESS_TOKEN);
}
function getRefreshToken() {
return localStorage.getItem(AUTH_KEYS.REFRESH_TOKEN);
}
function clearTokens() {
localStorage.removeItem(AUTH_KEYS.ACCESS_TOKEN);
localStorage.removeItem(AUTH_KEYS.REFRESH_TOKEN);
// In-memory storage for the refresh token to avoid persisting it in localStorage.
let inMemoryRefreshToken = null;
// ============================================================================
// Token Management
// ============================================================================
function storeTokens(accessToken, refreshToken) {
// Store the access token in sessionStorage so it is not persisted across browser sessions.
if (accessToken) {
sessionStorage.setItem(AUTH_KEYS.ACCESS_TOKEN, accessToken);
} else {
sessionStorage.removeItem(AUTH_KEYS.ACCESS_TOKEN);
}
// Keep the refresh token only in memory; in production it should be handled via httpOnly cookies.
inMemoryRefreshToken = refreshToken || null;
}
function getAccessToken() {
return sessionStorage.getItem(AUTH_KEYS.ACCESS_TOKEN);
}
function getRefreshToken() {
return inMemoryRefreshToken;
}
function clearTokens() {
sessionStorage.removeItem(AUTH_KEYS.ACCESS_TOKEN);
inMemoryRefreshToken = null;

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +31
@Test
@DisplayName("should be instantiable")
void shouldBeInstantiable() {
assertThat(customOAuth2UserService).isNotNull();
}

@Test
@DisplayName("should extend DefaultOAuth2UserService")
void shouldExtendDefaultOAuth2UserService() {
assertThat(customOAuth2UserService)
.isInstanceOf(org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService.class);
}
}
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test coverage for CustomOAuth2UserService is minimal. The class contains important logic for fetching GitHub emails and enriching user data, including error handling and fallback logic, but the tests only verify that the class is instantiable and extends the correct parent class. Add tests to cover the enrichGitHubUser method, email fetching logic, and error scenarios to ensure this critical authentication component works correctly.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +16
eclipse.preferences.version=1
org.eclipse.jdt.core.classpath.outputOverlappingAnotherSource=ignore
org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore
org.eclipse.jdt.core.compiler.annotation.nonnull=javax.annotation.Nonnull
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=javax.annotation.ParametersAreNonnullByDefault
org.eclipse.jdt.core.compiler.annotation.nullable=javax.annotation.Nullable
org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled
org.eclipse.jdt.core.compiler.codegen.targetPlatform=25
org.eclipse.jdt.core.compiler.compliance=25
org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning
org.eclipse.jdt.core.compiler.problem.nullReference=warning
org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning
org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning
org.eclipse.jdt.core.compiler.problem.syntacticNullAnalysisForFields=enabled
org.eclipse.jdt.core.compiler.processAnnotations=enabled
org.eclipse.jdt.core.compiler.source=25
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eclipse-specific configuration files (.settings/, .project) are being added to version control. These IDE-specific files typically should not be committed as they can cause conflicts between developers using different IDEs or IDE versions. Consider adding these to .gitignore instead and letting each developer configure their own IDE settings.

Copilot uses AI. Check for mistakes.
private static final Logger logger = LoggerFactory.getLogger(CustomOAuth2UserService.class);
private static final String GITHUB_EMAILS_URL = "https://api.github.com/user/emails";

private final RestTemplate restTemplate = new RestTemplate();
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new RestTemplate instance is created as a field in this Spring component without proper configuration. RestTemplate should ideally be configured as a Spring bean with proper error handling, timeouts, and connection pooling. Consider injecting a configured RestTemplate bean instead of creating a new instance here, or at minimum configure timeouts to prevent hanging requests if GitHub's API is slow or unresponsive.

Copilot uses AI. Check for mistakes.
Comment on lines +51 to +53
org.springframework.web: INFO
org.hibernate.SQL: INFO
org.hibernate.orm.jdbc.bind: INFO
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logging levels in application-local.yml have been changed from DEBUG to INFO for Spring Web, Hibernate SQL, and JDBC bindings. While this reduces log verbosity, the change is not mentioned in the PR description. If this is intentional for the bundled frontend, it should be documented. Consider whether these debugging features should remain enabled in the local profile to help with development and troubleshooting.

Suggested change
org.springframework.web: INFO
org.hibernate.SQL: INFO
org.hibernate.orm.jdbc.bind: INFO
org.springframework.web: DEBUG
org.hibernate.SQL: DEBUG
org.hibernate.orm.jdbc.bind: DEBUG

Copilot uses AI. Check for mistakes.
const confirmInput = document.getElementById('confirm-password');
const strengthBar = document.getElementById('password-strength');
const hint = document.getElementById('password-hint');
const matchWarning = document.getElementById('password-match');
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable matchWarning.

Suggested change
const matchWarning = document.getElementById('password-match');

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants